fix(cursor): seed default cursor with enabled=false to prevent overlay process leak - #1900
fix(cursor): seed default cursor with enabled=false to prevent overlay process leak#1900chxrro wants to merge 3 commits into
Conversation
…y leak The default cursor was seeded with enabled=true in three places: - CursorRegistry::new() (platform-macos/src/cursor/state.rs) - overlay::init() (platform-macos/src/cursor/overlay.rs) - RenderStateCore::new() hardcoded visible=true (cursor-overlay/src/render_state.rs) This caused every anonymous (non-session) tool call to render a cursor overlay that was never cleaned up — the "default" key is explicitly guarded against removal. The documented contract is "without a session, actions run cursor-less," but the hardcoded visible=true bypassed the config entirely. Fixes: 1. state.rs: seed default with enabled=false 2. overlay.rs: seed default render state with enabled=false 3. render_state.rs: derive visible from cfg.enabled instead of hardcoding true Without a declared session, the overlay window still exists (AppKit NSWindow lifecycle) but the default cursor draws nothing — paint_cursor early-returns on !visible, and animate_cursor_to skips on !cfg.enabled. Session cursors work normally via set_agent_cursor_enabled / start_session. Closes #1777
|
Someone is attempting to deploy a commit to the Cua Team on Vercel. A member of the Team first needs to authorize it. |
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThree files are updated to ensure the seeded ChangesDefault cursor disabled at initialization
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@libs/cua-driver/rust/crates/cursor-overlay/src/render_state.rs`:
- Around line 99-100: The issue is that visible and cfg.enabled can diverge
after initialization. While RenderStateCore::new initializes visible from
cfg.enabled, the OverlayCommand::SetEnabled handler only mutates visible without
updating cfg.enabled. This causes the animation gating check in
animate_cursor_to (which uses cfg.enabled) to become stale after toggles. Fix
this by ensuring that whenever the enabled state is toggled in the SetEnabled
handler at line 115, both visible and cfg.enabled are updated together to keep
them synchronized.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 19234aa1-ae73-4e06-aee0-9666d9572c4c
📒 Files selected for processing (3)
libs/cua-driver/rust/crates/cursor-overlay/src/render_state.rslibs/cua-driver/rust/crates/platform-macos/src/cursor/overlay.rslibs/cua-driver/rust/crates/platform-macos/src/cursor/state.rs
| let visible = cfg.enabled; | ||
| Self { |
There was a problem hiding this comment.
Keep cfg.enabled and visible in sync to avoid stale enablement checks.
RenderStateCore::new now initializes visible from cfg.enabled, but runtime toggles (OverlayCommand::SetEnabled) still mutate only visible. Since animate_cursor_to gates on rs.core.cfg.enabled in platform-macos/src/cursor/overlay.rs (Line 308), enable/disable state can diverge and break animation gating after toggles.
Suggested fix
diff --git a/libs/cua-driver/rust/crates/cursor-overlay/src/render_state.rs b/libs/cua-driver/rust/crates/cursor-overlay/src/render_state.rs
@@
OverlayCommand::SetEnabled(v) => {
self.visible = v;
+ self.cfg.enabled = v;
true
}Also applies to: 115-115
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@libs/cua-driver/rust/crates/cursor-overlay/src/render_state.rs` around lines
99 - 100, The issue is that visible and cfg.enabled can diverge after
initialization. While RenderStateCore::new initializes visible from cfg.enabled,
the OverlayCommand::SetEnabled handler only mutates visible without updating
cfg.enabled. This causes the animation gating check in animate_cursor_to (which
uses cfg.enabled) to become stale after toggles. Fix this by ensuring that
whenever the enabled state is toggled in the SetEnabled handler at line 115,
both visible and cfg.enabled are updated together to keep them synchronized.
Problem
Every anonymous (non-session) CUA tool call spawns a cursor overlay process
that is never cleaned up, causing 30+ zombie processes after moderate use.
Root cause: the default cursor is seeded in three places with a hardcoded
assumption that it should always render:
CursorRegistry::new()seeds"default"withenabled: trueoverlay::init()seeds default render state from the launch configRenderStateCore::new()hardcodesvisible: true, ignoringcfg.enabledentirely — the documented contract says "without asession, actions run cursor-less" but the code always renders
The
"default"key is explicitly guarded against removal (both inCursorRegistry::remove()andoverlay::apply_msg()), so theseoverlays accumulate until daemon restart.
Fix (3 files, 24 lines)
platform-macos/src/cursor/state.rs— seed default cursorregistry entry with
enabled: falseplatform-macos/src/cursor/overlay.rs— seed default renderstate with
enabled: falsecursor-overlay/src/render_state.rs— derivevisiblefromcfg.enabledinstead of hardcodingtrueThe overlay NSWindow still exists (AppKit lifecycle unchanged) but the
default cursor draws nothing —
paint_cursorearly-returns on!visible, andanimate_cursor_toskips on!cfg.enabled.Session cursors still work normally via
set_agent_cursor_enabled/start_session.Verification
default_cursor_starts_disabledverifies the seedprocess (vs 30+ before)
Closes #1777
Summary by CodeRabbit
Bug Fixes